-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(oauth): Implement OAuth 2.0 Device Authorization Flow (RFC 8628) #105675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
abc32ef to
a25ea15
Compare
|
This PR has a migration; here is the generated SQL for for --
-- Create model ApiDeviceCode
--
CREATE TABLE "sentry_apidevicecode" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "device_code" varchar(64) NOT NULL UNIQUE, "user_code" varchar(16) NOT NULL UNIQUE, "application_id" bigint NOT NULL, "user_id" bigint NULL, "organization_id" bigint NULL, "scope_list" text[] NOT NULL, "expires_at" timestamp with time zone NOT NULL, "status" varchar(20) NOT NULL, "date_added" timestamp with time zone NOT NULL);
ALTER TABLE "sentry_apidevicecode" ADD CONSTRAINT "sentry_apidevicecode_application_id_cf8361a8_fk_sentry_ap" FOREIGN KEY ("application_id") REFERENCES "sentry_apiapplication" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_apidevicecode" VALIDATE CONSTRAINT "sentry_apidevicecode_application_id_cf8361a8_fk_sentry_ap";
ALTER TABLE "sentry_apidevicecode" ADD CONSTRAINT "sentry_apidevicecode_user_id_ec448031_fk_auth_user_id" FOREIGN KEY ("user_id") REFERENCES "auth_user" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_apidevicecode" VALIDATE CONSTRAINT "sentry_apidevicecode_user_id_ec448031_fk_auth_user_id";
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_device_code_6d4da78d_like" ON "sentry_apidevicecode" ("device_code" varchar_pattern_ops);
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_user_code_90955a60_like" ON "sentry_apidevicecode" ("user_code" varchar_pattern_ops);
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_application_id_cf8361a8" ON "sentry_apidevicecode" ("application_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_user_id_ec448031" ON "sentry_apidevicecode" ("user_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_organization_id_c2717dcf" ON "sentry_apidevicecode" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_expires_at_1f1b6c16" ON "sentry_apidevicecode" ("expires_at"); |
| if scopes: | ||
| pending_scopes = set(scopes) | ||
| matched_sets = set() | ||
| for scope_set in settings.SENTRY_SCOPE_SETS: | ||
| for scope, description in scope_set: | ||
| if scope_set in matched_sets and scope in pending_scopes: | ||
| pending_scopes.remove(scope) | ||
| elif scope in pending_scopes: | ||
| permissions.append(description) | ||
| matched_sets.add(scope_set) | ||
| pending_scopes.remove(scope) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
actually this shows almost 6 hours, so i guess the cost includes the previous cost... |
Align with industry conventions (GitHub, Google) by using the shorter /oauth/device/code path instead of /oauth/device_authorization. - /oauth/device/code - device authorization endpoint (POST) - /oauth/device - user verification page (GET/POST)
Move the device code status check before authorization creation and wrap both operations in a transaction. This prevents orphaned authorizations if the device code update fails (e.g., due to race condition where another request already processed the device code). Previously, the authorization was created first, then the device code status was checked. If the status check failed, the authorization would persist even though no token could be issued.
Wrap token creation and device code deletion in a transaction to prevent duplicate tokens if delete fails after token creation. This follows the same pattern as grant_exchanger.py.
PostgreSQL aborts transactions on IntegrityError, preventing subsequent DB operations. Move try/except outside atomic block to allow the scope-merging code in the except block to run correctly.
Add expiration check after re-fetching device code inside the lock to prevent a race condition where a code could expire during lock wait.
Migration 1015 was taken by backfill_self_hosted_sentry_app_emails on master, so rename to 1016 and update dependencies.
- Extract magic numbers into named constants (DEVICE_CODE_BYTES, USER_CODE_GROUP_LENGTH) - Add user_code to ApiDeviceCode.__str__ for better debugging - Add "You can now close this tab" UX message to completion page - Use constants in _normalize_user_code instead of hardcoded values
2426197 to
836a149
Compare
|
This PR has a migration; here is the generated SQL for for --
-- Create model ApiDeviceCode
--
CREATE TABLE "sentry_apidevicecode" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "device_code" varchar(64) NOT NULL UNIQUE, "user_code" varchar(16) NOT NULL UNIQUE, "application_id" bigint NOT NULL, "user_id" bigint NULL, "organization_id" bigint NULL, "scope_list" text[] NOT NULL, "expires_at" timestamp with time zone NOT NULL, "status" varchar(20) NOT NULL, "date_added" timestamp with time zone NOT NULL);
ALTER TABLE "sentry_apidevicecode" ADD CONSTRAINT "sentry_apidevicecode_application_id_cf8361a8_fk_sentry_ap" FOREIGN KEY ("application_id") REFERENCES "sentry_apiapplication" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_apidevicecode" VALIDATE CONSTRAINT "sentry_apidevicecode_application_id_cf8361a8_fk_sentry_ap";
ALTER TABLE "sentry_apidevicecode" ADD CONSTRAINT "sentry_apidevicecode_user_id_ec448031_fk_auth_user_id" FOREIGN KEY ("user_id") REFERENCES "auth_user" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_apidevicecode" VALIDATE CONSTRAINT "sentry_apidevicecode_user_id_ec448031_fk_auth_user_id";
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_device_code_6d4da78d_like" ON "sentry_apidevicecode" ("device_code" varchar_pattern_ops);
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_user_code_90955a60_like" ON "sentry_apidevicecode" ("user_code" varchar_pattern_ops);
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_application_id_cf8361a8" ON "sentry_apidevicecode" ("application_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_user_id_ec448031" ON "sentry_apidevicecode" ("user_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_organization_id_c2717dcf" ON "sentry_apidevicecode" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_apidevicecode_expires_at_1f1b6c16" ON "sentry_apidevicecode" ("expires_at"); |
Remove `# for type checker` and similar comments that aren't used elsewhere in the codebase. The assertions are self-explanatory.
Restructure the code generation retry loop to avoid unreachable code after the loop. The previous structure had a final raise statement that could never be reached since the loop always either returned on success or raised on the last failed attempt. The new structure: - Tracks the last IntegrityError for debugging context - Raises after the loop completes (which only happens when all retries fail) - Chains the original error using `from last_error` for better traceability
- Use StrEnum for DeviceCodeStatus for better type safety - Remove redundant pass statement in UserCodeCollisionError - Simplify scope parsing with inline conditional - Remove dead code assignment for device_code.organization_id
0278f98 to
df8eae2
Compare
POST requests to /oauth/device/code/ were returning 404 because Django's APPEND_SLASH only redirects GET requests. Adding trailing slash for consistency with other OAuth endpoints.
…106169) ## Summary Fixes the OAuth 2.0 Device Authorization Grant implementation to properly support **public clients** as required by [RFC 8628](https://datatracker.ietf.org/doc/html/rfc8628). ## Problem The current token endpoint requires `client_secret` for ALL grant types, including `device_code`. This breaks the fundamental design of the device flow, which is explicitly designed for public clients (CLIs, native apps, IoT devices) that **cannot securely store secrets**. ### What the RFC Says **[RFC 8628 §5.6 - Non-Confidential Clients](https://datatracker.ietf.org/doc/html/rfc8628#section-5.6):** > Device clients are generally incapable of maintaining the confidentiality of their credentials, as users in possession of the device can reverse-engineer it and extract the credentials. **Therefore, unless additional measures are taken, they should be treated as public clients** (as defined by Section 2.1 of [RFC6749]). **[RFC 8628 §3.4 - Device Access Token Request](https://datatracker.ietf.org/doc/html/rfc8628#section-3.4):** > `client_id` - REQUIRED if the client is **not authenticating** with the authorization server as described in Section 3.2.1. of [RFC6749]. **[RFC 6749 §2.1 - Client Types](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1):** > **public** - Clients incapable of maintaining the confidentiality of their credentials (e.g., clients executing on the device used by the resource owner, such as an installed native application or a web browser-based application) ## Solution - Move `grant_type` validation **before** credential check (needed to determine auth requirements) - For `device_code` grant: only require `client_id` (public client mode per RFC 8628 §5.6) - If `client_secret` is provided: still validate it (supports confidential clients that choose to authenticate) - Other grant types (`authorization_code`, `refresh_token`): unchanged, still require `client_id` + `client_secret` ## Changes | File | Change | |------|--------| | `src/sentry/web/frontend/oauth_token.py` | Allow public client auth for device_code grant | | `tests/sentry/web/frontend/test_oauth_token.py` | Added 5 new tests for public client support | ## New Tests 1. `test_public_client_success` - Public client can exchange approved device code 2. `test_public_client_invalid_client_id` - Invalid client_id rejected with 401 3. `test_public_client_missing_client_id` - Missing client_id rejected with 401 4. `test_public_client_authorization_pending` - Polling works for public clients 5. `test_confidential_client_wrong_secret_rejected` - Wrong secret still rejected when provided ## Security Considerations This change is **RFC-compliant** and does not reduce security: 1. **Device code binding**: The `device_code` is bound to the `application` at creation time, so a public client can only poll for tokens for its own application 2. **User authorization**: The user must still explicitly approve the request via the browser 3. **Confidential clients still supported**: If a client provides `client_secret`, we validate it 4. **Rate limiting**: Existing rate limiting on device code polling remains in place ## Related - Original device flow implementation: #105675 - RFC 8628: https://datatracker.ietf.org/doc/html/rfc8628 --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Summary
Adds support for the OAuth 2.0 Device Authorization Grant (RFC 8628), enabling headless clients (CLIs, CI/CD pipelines, Docker containers) to obtain OAuth tokens by having users authorize on a separate device with a browser.
Key Components
POST /oauth/device_authorization) - Returns device_code, user_code, and verification URLsGET/POST /oauth/device) - Where users enter the code and approve/deny accessurn:ietf:params:oauth:grant-type:device_codegrant typeFlow
POST /oauth/device_authorizationdevice_code(secret) anduser_code(human-readable likeABCD-EFGH)user_codeandverification_urito userPOST /oauth/tokenwithdevice_codeRefs #99002
Refs getsentry/sentry-mcp#546